Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to Humble/Garden on 22.04 #111

Merged
merged 66 commits into from
Jan 21, 2023
Merged

Switch to Humble/Garden on 22.04 #111

merged 66 commits into from
Jan 21, 2023

Conversation

andermi
Copy link
Collaborator

@andermi andermi commented Jan 9, 2023

Get tests passing

merge along with:
osrf/mbari_wec_utils#38

chapulina and others added 30 commits August 17, 2022 14:52
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Dharini Dutia <dharini@openrobotics.org>
Signed-off-by: Dharini Dutia <dharini@openrobotics.org>
- Update library and namespaces for ign -> gz migration.
- Use `python -m em` rather than `empy` for macOS support.
- Update dependencies to ros_gz_bridge and ros_gz_sim

Signed-off-by: Rhys Mainwaring <rhys.mainwaring@me.com>
Signed-off-by: Dharini Dutia <dharini@openrobotics.org>
Signed-off-by: Dharini Dutia <dharini@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
@andermi
Copy link
Collaborator Author

andermi commented Jan 19, 2023

@mabelzhang @mjcarroll I think it works with cyclonedds?

@andermi
Copy link
Collaborator Author

andermi commented Jan 19, 2023

rerunning the jobs

@mabelzhang
Copy link
Collaborator

mabelzhang commented Jan 19, 2023

The PR tests succeeded, but the push tests failed. Is it flaky?

	 35 - launch_pc_commands_ros_feedback.launch.py (Failed)

I see it passing on the PR one. Maybe rerun?

@andermi
Copy link
Collaborator Author

andermi commented Jan 19, 2023

Is it flaky?

yeah seems to be flaky. I ran the tests once and everything passed. I re-ran them and this one failed with a timeout. On one of the tests I fixed a timeout by adding a sleep at the end after rclcpp shutdown and then launch_testing picked up that the process ended. I was going to try the same with this one.

@mabelzhang
Copy link
Collaborator

Sounds promising

Signed-off-by: Michael Anderson <anderson@mbari.org>
@andermi
Copy link
Collaborator Author

andermi commented Jan 20, 2023

seems good?

@andermi
Copy link
Collaborator Author

andermi commented Jan 20, 2023

not sure how to remove the (fortress, galactic) required status

@quarkytale
Copy link
Contributor

@andermi @mabelzhang it seems like we can either add a branch filter in the workflow or just skip the workflow run temporarily, see here. What would be better?

@andermi
Copy link
Collaborator Author

andermi commented Jan 20, 2023

@quarkytale @mabelzhang well, I'm not sure why we have a check for fortress/galactic in this PR at all. Can we just get rid of that check? I can't find it in the workflow...

@andermi andermi requested a review from mabelzhang January 20, 2023 21:18
@mabelzhang
Copy link
Collaborator

mabelzhang commented Jan 20, 2023

Should we remove the extra rosbag things and the extra timeouts originally added for the default RMW, and retest to see if we can do without them? Other than following clean software engineering practice, the practical benefit is that if CI doesn't need to sleep extra time, CI will be quicker.

Signed-off-by: Michael Anderson <anderson@mbari.org>
@andermi
Copy link
Collaborator Author

andermi commented Jan 21, 2023

@mabelzhang I stopped generating rosbags for the two tests I enabled them for. There are only 1 second sleeps in two files I added right before an exe exits. I needed those even with the rmw change to cyclonedds.

Copy link
Collaborator

@mabelzhang mabelzhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I've looked at all the files. One minor style comment.
Re timing - okay, sounds good. I see the rosbag being disabled.

I'm approving because I'm done for the week. But there are 2 followup tickets we need to do below.

This test does seem to be flaky. We can merge now because we know it is flaky, but we should open an issue ticket to track it, and so that we know it's flaky in the future. Either we can try in another PR to add more timeout, or disable it so that we're not always merging on red CI.

	 35 - launch_pc_commands_ros_feedback.launch.py (Failed)

Don't forget to change the github workflow line 31 back to main in a followup PR.

buoy_tests/worlds/TestMachine.sdf Outdated Show resolved Hide resolved
andermi and others added 2 commits January 20, 2023 21:39
Co-authored-by: Mabel Zhang <mabel.m.zhang@gmail.com>
@andermi andermi enabled auto-merge (squash) January 21, 2023 03:50
@andermi andermi disabled auto-merge January 21, 2023 06:29
@quarkytale quarkytale enabled auto-merge (squash) January 21, 2023 09:37
@andermi
Copy link
Collaborator Author

andermi commented Jan 21, 2023

It seems to be failing more consistently, and the only error is a print of FAIL

I'll try to debug it a bit more on Monday

@quarkytale quarkytale merged commit 7723d27 into main Jan 21, 2023
@quarkytale quarkytale deleted the chapulina/humble_garden branch January 21, 2023 20:18
@quarkytale quarkytale restored the chapulina/humble_garden branch January 22, 2023 21:41
@quarkytale
Copy link
Contributor

Looks like it passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants